optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode#2966
Open
contrueCT wants to merge 6 commits intoapache:masterfrom
Open
optimize(api): disable GraphSpaceAPI and ManagerAPI in standalone mode#2966contrueCT wants to merge 6 commits intoapache:masterfrom
contrueCT wants to merge 6 commits intoapache:masterfrom
Conversation
- Add public isUsePD() accessor to GraphManager to expose PD status - Add checkPdModeEnabled() helper in API base class - Call checkPdModeEnabled() in all public methods of GraphSpaceAPI (list/get/create/manage/delete) - Call checkPdModeEnabled() in all public methods of ManagerAPI (createManager/delete/list/checkRole/getRolesInGs) - Returns HTTP 400 with message 'GraphSpace management is not supported in standalone mode' - Add standalone-mode rejection tests in GraphSpaceApiTest and ManagerApiTest
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #2964 by disabling GraphSpaceAPI and ManagerAPI endpoints in standalone (non-PD) mode, returning a friendly HTTP 400 error instead of allowing logic that doesn't apply outside of PD mode.
Changes:
- Added
isUsePD()public accessor toGraphManagerto expose PD status - Added
checkPdModeEnabled()static helper inAPIbase class, called at the start of all public methods inGraphSpaceAPIandManagerAPI - Added standalone-mode rejection tests in
GraphSpaceApiTestandManagerApiTest
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
GraphManager.java |
Exposes isUsePD() as a public method |
API.java |
Adds checkPdModeEnabled() helper throwing HTTP 400 in non-PD mode |
GraphSpaceAPI.java |
Guards all public endpoints with checkPdModeEnabled() |
ManagerAPI.java |
Guards all public endpoints with checkPdModeEnabled() |
GraphSpaceApiTest.java |
Tests that all GraphSpaceAPI endpoints return 400 in standalone mode |
ManagerApiTest.java |
Tests that all ManagerAPI endpoints return 400 in standalone mode |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java
Show resolved
Hide resolved
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java
Outdated
Show resolved
Hide resolved
imbajin
reviewed
Mar 10, 2026
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java
Show resolved
Hide resolved
imbajin
reviewed
Mar 10, 2026
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ManagerApiTest.java
Outdated
Show resolved
Hide resolved
imbajin
reviewed
Mar 11, 2026
...erver/hugegraph-test/src/main/java/org/apache/hugegraph/api/GraphSpaceApiStandaloneTest.java
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/space/GraphSpaceAPI.java
Show resolved
Hide resolved
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java
Show resolved
Hide resolved
...h-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ManagerApiStandaloneTest.java
Show resolved
Hide resolved
...erver/hugegraph-test/src/main/java/org/apache/hugegraph/api/GraphSpaceApiStandaloneTest.java
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/BaseApiTest.java
Show resolved
Hide resolved
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java
Show resolved
Hide resolved
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose of the PR
Main Changes
The issue requires GraphSpaceAPI to return a friendly response in standalone mode. During code investigation, I found that ManagerAPI has a similar problem: in standalone mode, manager.graphSpace(name) returns a fake new GraphSpace("DEFAULT"), which causes ManagerAPI.validGraphSpace() to pass incorrectly. As a result, ManagerAPI can still enter logic that is not actually supported in standalone mode, leading to unfriendly errors or behavior that does not match the API semantics. For consistency and correctness, I applied the same standalone-mode guard to ManagerAPI as well.
Verifying these changes
Does this PR potentially affect the following parts?
Documentation Status
Doc - TODODoc - DoneDoc - No Need